-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add per_page parameter to page #301
Conversation
src/lib/ui/ExtensionsList.svelte
Outdated
let showCategories = true; | ||
|
||
// Get the query from the URL | ||
function getQueryLimit(): number | null { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: null is not authorized (I was assuming we got a lint rule for that)
---> undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woops! old habits
<ExtensionsByCategory {extensionByCategoryInfo} /> | ||
{/each} | ||
{#if showCategories} | ||
{#each extensionsByCategories as extensionByCategoryInfo} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (non blocking) but not sure why: looks like we have existing tab characters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, looks like pnpm format:fix
does not actually do svelte files too
6f0bf0c
to
746d7ea
Compare
src/lib/ui/ExtensionsList.svelte
Outdated
import ExtensionsByCategory from './ExtensionsByCategory.svelte'; | ||
|
||
const { extensionsByCategories }: { extensionsByCategories: ExtensionByCategoryInfo[] } = $props(); | ||
export let extensionsByCategories: ExtensionByCategoryInfo[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you reverting to svelte 4?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will fix, bad habit using svelte 4 for everything haha!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name proposition: per_page (as per GitHub API) query is to vague for me
Updated to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Title and description to be updated
Updated the PR commit title and description + the actual commit now. Thanks! |
Adds per_page parameter to the page. If you pass in ?per_page=4 into the URL, it'll show only the last 4 updated extensions. Open for suggestions on other names for it. Signed-off-by: Charlie Drage <[email protected]>
Signed-off-by: Charlie Drage <[email protected]>
chore: add per_page parameter to page
Adds per_page parameter to the page.
If you pass in ?per_page=4 into the URL, it'll show only the last 4 updated
extensions.
Open for suggestions on other names for it.
Closes #192
Signed-off-by: Charlie Drage [email protected]